Skip to content

Conversation

@ptiurin
Copy link
Contributor

@ptiurin ptiurin commented Aug 22, 2025

This pull request introduces a significant refactor of the token caching mechanism for authentication in the Firebolt Python SDK. The main changes involve replacing the previous filesystem-based token cache with a new, encrypted, file-based cache system, updating the Auth classes to use this new cache, and removing legacy token storage code. The update also ensures that account names are consistently set in authentication objects during connection setup.

Token caching and storage refactor:

  • Introduced a new FileBasedCache class in src/firebolt/utils/cache.py that provides encrypted, persistent, file-based caching for authentication tokens and connection info, replacing the old TokenSecureStorage system. This includes logic for OS-specific cache directories, encryption, and expiry handling.
  • Updated the Auth class in src/firebolt/client/auth/base.py to use the new _firebolt_cache for token retrieval and storage, including adding an account property to ensure tokens are cached per-account.
  • Removed all references to the legacy TokenSecureStorage and related code from the authentication classes (UsernamePassword, ServiceAccount, ClientCredentials), and deleted the compatibility import in src/firebolt/common/token_storage.py.

Connection setup improvements:

  • Ensured that the account_name is consistently assigned to the auth object in both async and sync connect functions, which is necessary for correct token caching and retrieval.

Data structure enhancements:

  • Enhanced the ConnectionInfo dataclass to include a token field and a __post_init__ method for robust deserialization of nested dataclasses from cache.

@ptiurin ptiurin changed the title working file caching refactor: merge token and url caching Aug 22, 2025
@ptiurin ptiurin changed the title refactor: merge token and url caching refactor(FIR-46254): merge token and url caching Aug 22, 2025
@sonarqubecloud
Copy link

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 6, 2025

@ptiurin ptiurin marked this pull request as ready for review November 19, 2025 11:10
@ptiurin ptiurin requested a review from a team as a code owner November 19, 2025 11:10
def _token_storage(self) -> Optional[TokenSecureStorage]:
"""Token filesystem cache storage.
def _get_cached_token(self) -> Tuple[Optional[str], Optional[int]]:
"""If caching is enabled, get token from cache.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am guessing we also return the validity of the token? What is the tuple return needed for

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ptiurin
Copy link
Contributor Author

ptiurin commented Nov 26, 2025

Created merge_caching_bkp in case some merge conflicts were not resolved correctly.

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants